[jules] ux: Complete skeleton loading for HomeScreen groups#323
[jules] ux: Complete skeleton loading for HomeScreen groups#323
Conversation
Replaced the basic `ActivityIndicator` on the mobile `HomeScreen` with a modern skeleton loading experience. - Created `mobile/components/ui/Skeleton.js` using `Animated` for a pulsing opacity effect. - Created `mobile/components/skeletons/HomeScreenSkeleton.js` to mirror the group card layout. - Integrated the skeleton into `HomeScreen.js`. - Updated tracking files (`todo.md`, `changelog.md`, `knowledge.md`). Co-authored-by: Devasy23 <110348311+Devasy23@users.noreply.github.com>
|
👋 Jules, reporting for duty! I'm here to lend a hand with this pull request. When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down. I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job! For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with New to Jules? Learn more at jules.google/docs. For security, I will only act on instructions from the user who triggered this task. |
✅ Deploy Preview for split-but-wiser canceled.
|
WalkthroughIntroduces a reusable animated Changes
Suggested reviewers
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #323 +/- ##
=======================================
Coverage ? 63.55%
=======================================
Files ? 21
Lines ? 2456
Branches ? 254
=======================================
Hits ? 1561
Misses ? 831
Partials ? 64
🚀 New features to boost your workflow:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.Jules/changelog.md:
- Around line 127-128: The changelog's "### Mobile" heading lacks a trailing
blank line causing markdownlint MD022; edit .Jules/changelog.md and insert a
single blank line immediately after the "### Mobile" heading (i.e., ensure there
is an empty line between the heading "### Mobile" and the following list item "-
Added reusable animated `Skeleton` primitive component...") so the heading is
separated from the content.
In `@mobile/components/skeletons/HomeScreenSkeleton.js`:
- Around line 10-14: Card.Title currently receives a React element for its title
prop (title={<Skeleton ... />}), which mismatches the prop signature; change it
to the render-function form like left uses (e.g., title={() => <Skeleton
width={120} height={18} borderRadius={4} />}) so Card.Title renders a node
correctly; update the Card.Title invocation in HomeScreenSkeleton.js to pass a
function for title that returns the Skeleton component.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 1f15b9fc-0eaa-4a55-a924-b10036d47386
📒 Files selected for processing (6)
.Jules/changelog.md.Jules/knowledge.md.Jules/todo.mdmobile/components/skeletons/HomeScreenSkeleton.jsmobile/components/ui/Skeleton.jsmobile/screens/HomeScreen.js
| ### Mobile | ||
| - Added reusable animated `Skeleton` primitive component in `mobile/components/ui/Skeleton.js`. |
There was a problem hiding this comment.
markdownlint MD022: add a blank line after the ### Mobile heading.
📝 Proposed fix
### Mobile
+
- Added reusable animated `Skeleton` primitive component in `mobile/components/ui/Skeleton.js`.📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| ### Mobile | |
| - Added reusable animated `Skeleton` primitive component in `mobile/components/ui/Skeleton.js`. | |
| ### Mobile | |
| - Added reusable animated `Skeleton` primitive component in `mobile/components/ui/Skeleton.js`. |
🧰 Tools
🪛 markdownlint-cli2 (0.22.0)
[warning] 127-127: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.Jules/changelog.md around lines 127 - 128, The changelog's "### Mobile"
heading lacks a trailing blank line causing markdownlint MD022; edit
.Jules/changelog.md and insert a single blank line immediately after the "###
Mobile" heading (i.e., ensure there is an empty line between the heading "###
Mobile" and the following list item "- Added reusable animated `Skeleton`
primitive component...") so the heading is separated from the content.
| <Card key={i} style={styles.card}> | ||
| <Card.Title | ||
| title={<Skeleton width={120} height={18} borderRadius={4} />} | ||
| left={() => <Skeleton width={40} height={40} borderRadius={20} />} | ||
| /> |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
react-native-paper 5.x Card.Title title prop type accepted values
💡 Result:
The title prop of Card.Title in react-native-paper 5.x accepts React.ReactNode values. This includes strings, numbers, JSX elements (like components), and other renderable React children. The TypeScript definition explicitly states: "Text for the title. Note that this will only accept a string or -based node," confirming compatibility with text-based React nodes.
Citations:
- 1: https://cdn.jsdelivr.net/npm/react-native-paper@5.1.2/lib/typescript/components/Card/CardTitle.d.ts
🏁 Script executed:
# First, check if the file exists and verify the code snippet
cat -n mobile/components/skeletons/HomeScreenSkeleton.js | head -20Repository: Devasy/splitwiser
Length of output: 975
🏁 Script executed:
# Find react-native-paper package.json to identify version
find . -name "package.json" -type f | head -5 | xargs grep -l "react-native-paper" 2>/dev/nullRepository: Devasy/splitwiser
Length of output: 82
🏁 Script executed:
# Check for Skeleton component definition in the repo
fd -t f "Skeleton" --extension js --extension ts --extension tsx mobile/Repository: Devasy/splitwiser
Length of output: 143
🏁 Script executed:
# Look for HomeScreen.js file to check the pattern at line 196
fd -t f "HomeScreen.js"Repository: Devasy/splitwiser
Length of output: 89
🏁 Script executed:
# Check the react-native-paper version
cat mobile/package.json | grep -A 2 -B 2 "react-native-paper"Repository: Devasy/splitwiser
Length of output: 241
🏁 Script executed:
# Examine the Skeleton component definition
cat mobile/components/ui/Skeleton.jsRepository: Devasy/splitwiser
Length of output: 1096
🏁 Script executed:
# Check HomeScreen.js line 196 and surrounding context
sed -n '190,205p' mobile/screens/HomeScreen.jsRepository: Devasy/splitwiser
Length of output: 646
🏁 Script executed:
# Check if node_modules has react-native-paper TypeScript definitions
find node_modules -path "*/react-native-paper/*/CardTitle.d.ts" 2>/dev/null | head -1 | xargs catRepository: Devasy/splitwiser
Length of output: 43
🏁 Script executed:
# Alternative: search for CardTitle types in node_modules
find node_modules/react-native-paper -name "*Card*" -type f | grep -E "\.(d\.ts|ts)$" | head -5Repository: Devasy/splitwiser
Length of output: 127
🏁 Script executed:
# Let's also check what HapticCard is in the codebase (different from Card.Title pattern)
rg -t ts -t tsx "HapticCard" mobile/ -B 2 -A 2 | head -30Repository: Devasy/splitwiser
Length of output: 88
🏁 Script executed:
# Let's search for how Card.Title is used elsewhere in the codebase
rg "Card\.Title" mobile/ -B 2 -A 4 -t jsRepository: Devasy/splitwiser
Length of output: 4271
🏁 Script executed:
# Check if there's a type definition file in the project itself
find . -name "*.d.ts" -type f | head -10Repository: Devasy/splitwiser
Length of output: 43
🏁 Script executed:
# Verify if there are any runtime issues or patterns in similar React Native projects
# Let's also check if the title prop is documented to accept render functions
rg -t js "title=\{.*=>.*\}" mobile/ | head -10Repository: Devasy/splitwiser
Length of output: 43
🏁 Script executed:
# Double-check if there's any other use of JSX elements as Card.Title props
rg "Card\.Title" mobile/ -A 2 | grep -E "title=\{<"Repository: Devasy/splitwiser
Length of output: 184
title prop receives a React element — use the render-function form instead.
react-native-paper Card.Title's title prop accepts string | ((props) => React.ReactNode) and internally wraps the string value in a <Text numberOfLines> component. Passing a non-text component like <Skeleton /> (an Animated.View) directly as title does not align with the prop's type signature and can cause layout issues in React Native. Use the render-function form, consistent with the left prop on line 13 and standard usage throughout the codebase.
🔧 Proposed fix
<Card.Title
- title={<Skeleton width={120} height={18} borderRadius={4} />}
+ title={() => <Skeleton width={120} height={18} borderRadius={4} />}
left={() => <Skeleton width={40} height={40} borderRadius={20} />}
/>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <Card key={i} style={styles.card}> | |
| <Card.Title | |
| title={<Skeleton width={120} height={18} borderRadius={4} />} | |
| left={() => <Skeleton width={40} height={40} borderRadius={20} />} | |
| /> | |
| <Card key={i} style={styles.card}> | |
| <Card.Title | |
| title={() => <Skeleton width={120} height={18} borderRadius={4} />} | |
| left={() => <Skeleton width={40} height={40} borderRadius={20} />} | |
| /> |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@mobile/components/skeletons/HomeScreenSkeleton.js` around lines 10 - 14,
Card.Title currently receives a React element for its title prop
(title={<Skeleton ... />}), which mismatches the prop signature; change it to
the render-function form like left uses (e.g., title={() => <Skeleton
width={120} height={18} borderRadius={4} />}) so Card.Title renders a node
correctly; update the Card.Title invocation in HomeScreenSkeleton.js to pass a
function for title that returns the Skeleton component.
1 participant
This submission replaces the default generic
ActivityIndicatoron the mobile HomeScreen with a new Skeleton loading state. It implements a reusable primitiveSkeletoncomponent leveraging React Native'sAnimatedlibrary and constructs aHomeScreenSkeletonmatching the standard layout ofHapticCarditems in the list. This enhancement improves the UX by making the loading process feel smoother and more polished. Tracking files have been updated, and temporary files utilized during Playwright testing have been strictly removed to maintain repository hygiene.PR created automatically by Jules for task 14560037939543496204 started by @Devasy23
Summary by CodeRabbit